Add centralized banner system with OS messaging support by Maffooch · Pull Request #14708 · DefectDojo/django-DefectDojo

Maffooch · 2026-04-18T02:10:01Z

Introduces a new Open Source Messaging banner that fetches markdown content from a GCS bucket, renders it with bleach-sanitized HTML, and
displays it via the existing additional_banners template loop. Cached for 1 hour; fetch failures silently yield no banner.
Replaces the DD_CREATE_CLOUD_BANNER / CREATE_CLOUD_BANNER setting and all cloud-banner-specific logic with a centralized additional_banners
context processor pattern. Both OS messages and product announcements now flow through the same rendering pipeline, each tagged with a source field
(os, product_announcement).
Migrates ProductAnnouncementManager from django.contrib.messages to session-based banners (_product_banners), which are popped by the context
processor and rendered in the unified template loop.
Removes the evaluate_pro_proposition Celery task/beat schedule and the create_announcement_banner initialization command logic.
Simplifies the add_announcement_to_new_user signal by removing cloud-banner conditional checks.
Adds 52 unit tests covering the OS message fetcher/parser and product announcement manager.

Fetches a markdown message from the DaaS-published GCS bucket, renders the bleached headline and optional expanded section through the existing additional_banners template loop. Cached for 1h; any fetch/parse failure silently yields no banner. No Django settings introduced — disabling the banner requires forking. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Single newlines in the expanded body now render as <br>, so authored markdown lays out multi-line. Module folded into the existing dojo/announcement/ app and test patch paths updated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Anchor-based toggle picked up Bootstrap alert link styles and a lingering focus outline after click, which showed as a stray glyph next to the caret. A plain <button type="button"> avoids link decoration entirely; focus outline and transition are also dropped so the caret flips instantly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Migrate all promotional messaging to the additional_banners context processor pattern. Product announcements now store banners in the session for rendering via the unified template loop. Each banner carries a source field (os, product_announcement) so downstream repos can filter by origin. - Remove DD_CREATE_CLOUD_BANNER setting and env var entirely - Repurpose ProductAnnouncementManager to use session-based banners - Remove evaluate_pro_proposition celery task and beat schedule - Remove create_announcement_banner from initialization command - Simplify announcement signal to remove cloud-specific logic - Add SHOW_PLG_LINK context variable for PLG menu item control - Rename os-banner-* CSS classes to generic banner-* classes - Add data-source attribute to banner template markup - Switch OS message bucket URL from dev to prod - Add 52 tests covering context processor and product announcements Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

dryrunsecurity · 2026-04-18T02:12:50Z

This pull request introduces potential Cross-Site Scripting risks: session-created banners are marked safe and later rendered with the template |safe filter without proper sanitization (product_announcements.py and template), while only the OS-fetched banner path uses bleach sanitization, leaving session-stored/interpolated content able to inject HTML/JS. Review and ensure all banner content is sanitized (or avoid mark_safe/|safe) before rendering to prevent XSS.

🔴 Potential Cross-Site Scripting in dojo/templates/base.html (drs_7163508e)

Vulnerability	Potential Cross-Site Scripting
Description	Template uses the safe filter to render banner.message and banner.expanded_html ({{ banner.messagesafe }}, {{ banner.expanded_htmlsafe }}), which disables Django auto-escaping. We must confirm whether the values in additional_banners are sanitized before being marked safe.

django-DefectDojo/dojo/templates/base.html

Lines 673 to 690 in 6a30549

    
           {% for banner in additional_banners %} 
        
               <div role="alert" class="announcement-banner alert alert-{{ banner.style }} show" 
        
                    data-source="{{ banner.source }}"> 
        
                   {{ banner.message|safe }}{% if banner.url %} <a href="{{ banner.url }}">{{ banner.link_text }}</a>{% endif %} 
        
                   {% if banner.expanded_html %} 
        
                       <button type="button" class="banner-toggle collapsed" 
        
                               data-toggle="collapse" 
        
                               data-target="#banner-expanded-{{ forloop.counter }}" 
        
                               aria-expanded="false" 
        
                               aria-controls="banner-expanded-{{ forloop.counter }}"> 
        
                           <i class="fa-solid fa-caret-down"></i> 
        
                       </button> 
        
                       <div id="banner-expanded-{{ forloop.counter }}" class="collapse banner-expanded"> 
        
                           {{ banner.expanded_html|safe }} 
        
                       </div> 
        
                   {% endif %} 
        
               </div> 
        
           {% endfor %}

🟠 Potential Cross-Site Scripting in dojo/context_processors.py (drs_1b98f186)

Vulnerability	Potential Cross-Site Scripting
Description	additional_banners may include data from two sources: (1) get_os_banner() which fetches remote Markdown, renders it to HTML via markdown.markdown and then sanitizes with bleach.clean into 'message' and 'expanded_html'; (2) arbitrary dicts stored in request.session by product_announcement.add_session_banner (message field created from str((message))) and later popped and added directly to context. The template renders banner.message and banner.expanded_html with the safe filter, bypassing Django auto-escaping. We must ensure those values are sanitized for the HTML context before marking safe; only the OS banner path applies bleach sanitization. Session-stored banners are not sanitized before being marked safe in the template, allowing user-controlled input to reach the rendering sink without escaping.

django-DefectDojo/dojo/context_processors.py

Lines 36 to 67 in 6a30549

    
                   "DOCUMENTATION_URL": settings.DOCUMENTATION_URL, 
        
                   "API_TOKENS_ENABLED": settings.API_TOKENS_ENABLED, 
        
                   "API_TOKEN_AUTH_ENDPOINT_ENABLED": settings.API_TOKEN_AUTH_ENDPOINT_ENABLED, 
        
                   "SHOW_PLG_LINK": True, 
        
                   # V3 Feature Flags 
        
                   "V3_FEATURE_LOCATIONS": settings.V3_FEATURE_LOCATIONS, 
        
               } 
        
               additional_banners = [] 
        
               if (os_banner := get_os_banner()) is not None: 
        
                   additional_banners.append({ 
        
                       "source": "os", 
        
                       "message": os_banner["message"], 
        
                       "style": "info", 
        
                       "url": "", 
        
                       "link_text": "", 
        
                       "expanded_html": os_banner["expanded_html"], 
        
                   }) 
        
               if hasattr(request, "session"): 
        
                   for banner in request.session.pop("_product_banners", []): 
        
                       additional_banners.append(banner) 
        
               if additional_banners: 
        
                   context["additional_banners"] = additional_banners 
        
               return context 
        
           def bind_system_settings(request): 
        
               """Load system settings and display warning if there's a database error."""

🟠 Potential Cross-Site Scripting in dojo/product_announcements.py (drs_bbe0400c)

Vulnerability	Potential Cross-Site Scripting
Description	mark_safe is used when building a session banner message: mark_safe(f"{self.base_message} {self.ui_outreach}") — this bypasses Django auto-escaping. If any interpolated content can be attacker-controlled, it would reach a template and be rendered without escaping, causing XSS.

django-DefectDojo/dojo/product_announcements.py

Lines 34 to 35 in 6a30549

    
               message=mark_safe(f"{self.base_message} {self.ui_outreach}"), 
        
           )

We've notified @mtesauro.

Comment to provide feedback on these findings.

Report false positive: @dryrunsecurity fp [FINDING ID] [FEEDBACK]
Report low-impact: @dryrunsecurity nit [FINDING ID] [FEEDBACK]

Example: @dryrunsecurity fp drs_90eda195 This code is not user-facing

All finding details can be found in the DryRun Security Dashboard.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

valentijnscholten

What is the reason for the server-side caching? Is it for client side performance or to save on outgoing traffic on the bucket? Wouldn't it be much easier to just let the browser load it straight from the bucket and configure the bucket with an appropriate Cache-Control header? This way it would also work if the server is inside some network that is not allowed to egress to GCS.

Maffooch · 2026-04-18T17:48:17Z

There is some formatting server side before the raw content is sent to the browser. Seems like it would be less performant to do that computation in the browser on every request rather than once every few through the cache

valentijnscholten

Yeah I don't understand why so much processing is needed. The content is in your control so I would expect it to be tailer made and would only need a nh3 or dompurify sanitize as assurance for OS users/admins.

mtesauro

Approved

Maffooch and others added 5 commits April 16, 2026 22:45

Remove unused import and add docstring to TestBannerDictSchema

62b9b92

Maffooch requested a review from mtesauro as a code owner April 18, 2026 02:10

github-actions Bot added settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR unittests ui labels Apr 18, 2026

Merge branch 'bugfix' into os-messaging-banner

39418cb

Fix ruff FURB189: use UserDict instead of dict subclass

6a30549

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Maffooch added this to the 2.57.2 milestone Apr 18, 2026

Maffooch requested review from Jino-T, blakeaowens, rossops and valentijnscholten April 18, 2026 02:37

valentijnscholten reviewed Apr 18, 2026

View reviewed changes

valentijnscholten approved these changes Apr 19, 2026

View reviewed changes

blakeaowens approved these changes Apr 20, 2026

View reviewed changes

rossops approved these changes Apr 20, 2026

View reviewed changes

mtesauro approved these changes Apr 20, 2026

View reviewed changes

mtesauro merged commit 43b2238 into bugfix Apr 20, 2026
159 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add centralized banner system with OS messaging support#14708

Add centralized banner system with OS messaging support#14708
mtesauro merged 7 commits intobugfixfrom
os-messaging-banner

Maffooch commented Apr 18, 2026 •

edited

Loading

Uh oh!

dryrunsecurity Bot commented Apr 18, 2026 •

edited

Loading

Uh oh!

valentijnscholten left a comment

Uh oh!

Maffooch commented Apr 18, 2026

Uh oh!

valentijnscholten left a comment

Uh oh!

mtesauro left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

Maffooch commented Apr 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dryrunsecurity Bot commented Apr 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

valentijnscholten left a comment

Choose a reason for hiding this comment

Uh oh!

Maffooch commented Apr 18, 2026

Uh oh!

valentijnscholten left a comment

Choose a reason for hiding this comment

Uh oh!

mtesauro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Maffooch commented Apr 18, 2026 •

edited

Loading

dryrunsecurity Bot commented Apr 18, 2026 •

edited

Loading